feat(node): override version metadata with morph-reth info#95
feat(node): override version metadata with morph-reth info#95
Conversation
Add build.rs and version.rs to morph-node so that --version reports morph-reth's own version, commit SHA, and build timestamp instead of the upstream reth fork's values. Also bump version to 0.2.2.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a Cargo build script using vergen to emit compile-time version variables, exposes a new Changes
Sequence Diagram(s)sequenceDiagram
participant Build as Build Script (vergen)
participant Cargo as Cargo / Compiler
participant Binary as morph-reth main
participant Version as morph_node::version
Build->>Cargo: emit cargo:rustc-env (MORPH_*, VERGEN_*)
Note right of Cargo: env! macros bake values into binary
Binary->>Version: init_version_metadata()
Version->>Binary: populate global RethCliVersionConsts
Binary->>Binary: continue signal setup & CLI parsing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/node/build.rs (1)
35-37: Avoid brittle OUT_DIR parsing for build profile.Line 36 depends on a fixed path shape and can panic unexpectedly. Prefer reading profile directly from build-script env instead of slicing
OUT_DIR.Proposed refactor
- let out_dir = env::var("OUT_DIR").unwrap(); - let profile = out_dir.rsplit(std::path::MAIN_SEPARATOR).nth(3).unwrap(); + let profile = env::var("PROFILE")?; println!("cargo:rustc-env=MORPH_BUILD_PROFILE={profile}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/build.rs` around lines 35 - 37, The current build script parses OUT_DIR into `profile` using `out_dir.rsplit(...).nth(3)` which is brittle and can panic; change the logic to read the build profile from the `PROFILE` environment variable (use `env::var("PROFILE")`) and fall back to a sensible default if missing, then use that value in the `println!("cargo:rustc-env=MORPH_BUILD_PROFILE={profile}")` call instead of the parsed `profile` from `out_dir`; update references to the `out_dir` and `profile` variables accordingly and remove the fragile rsplit usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/node/build.rs`:
- Around line 35-37: The current build script parses OUT_DIR into `profile`
using `out_dir.rsplit(...).nth(3)` which is brittle and can panic; change the
logic to read the build profile from the `PROFILE` environment variable (use
`env::var("PROFILE")`) and fall back to a sensible default if missing, then use
that value in the `println!("cargo:rustc-env=MORPH_BUILD_PROFILE={profile}")`
call instead of the parsed `profile` from `out_dir`; update references to the
`out_dir` and `profile` variables accordingly and remove the fragile rsplit
usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa2b4556-3665-49f9-a1e3-da86a216701b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlbin/morph-reth/src/main.rscrates/node/Cargo.tomlcrates/node/build.rscrates/node/src/lib.rscrates/node/src/version.rs
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/node/build.rs (2)
40-41: Avoid parsingOUT_DIRto get build profileLine 41 depends on Cargo path shape and can panic. Prefer
PROFILE, which Cargo already provides for build scripts.Proposed refactor
- let out_dir = env::var("OUT_DIR").unwrap(); - let profile = out_dir.rsplit(std::path::MAIN_SEPARATOR).nth(3).unwrap(); + let profile = env::var("PROFILE")?; println!("cargo:rustc-env=MORPH_BUILD_PROFILE={profile}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/build.rs` around lines 40 - 41, The build script currently derives the Cargo build profile by parsing OUT_DIR (env::var("OUT_DIR") and the local variable profile), which relies on Cargo's directory layout and can panic; change it to read the PROFILE environment variable provided to build scripts (env::var("PROFILE")) and use that value in place of the parsed profile so you no longer depend on splitting OUT_DIR; update any uses of the local profile variable accordingly and handle the Result (e.g., unwrap_or or expect) consistently.
29-39: Use one short-SHA length consistentlyLine 29 uses 7 chars, but Line 38 exports 8 chars. This can produce inconsistent metadata across fields.
Proposed cleanup
- let sha_short = &sha[0..7]; + let sha_short = &sha[..8]; @@ - println!("cargo:rustc-env=VERGEN_GIT_SHA_SHORT={}", &sha[..8]); + println!("cargo:rustc-env=VERGEN_GIT_SHA_SHORT={sha_short}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/node/build.rs` around lines 29 - 39, The code slices the git SHA with two different lengths (sha_short = &sha[0..7] but prints &sha[..8]), causing inconsistent metadata; update the logic to compute a single sha_short and reuse it everywhere (use sha_short for the not_on_tag comparison and for the VERGEN_GIT_SHA_SHORT export) so both checks/outputs use the identical substring, and ensure any format! or println! references (e.g., VERGEN_GIT_DESCRIBE check, MORPH_VERSION_SUFFIX calculation, and VERGEN_GIT_SHA_SHORT export) refer to that single sha_short symbol.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/node/build.rs`:
- Around line 40-41: The build script currently derives the Cargo build profile
by parsing OUT_DIR (env::var("OUT_DIR") and the local variable profile), which
relies on Cargo's directory layout and can panic; change it to read the PROFILE
environment variable provided to build scripts (env::var("PROFILE")) and use
that value in place of the parsed profile so you no longer depend on splitting
OUT_DIR; update any uses of the local profile variable accordingly and handle
the Result (e.g., unwrap_or or expect) consistently.
- Around line 29-39: The code slices the git SHA with two different lengths
(sha_short = &sha[0..7] but prints &sha[..8]), causing inconsistent metadata;
update the logic to compute a single sha_short and reuse it everywhere (use
sha_short for the not_on_tag comparison and for the VERGEN_GIT_SHA_SHORT export)
so both checks/outputs use the identical substring, and ensure any format! or
println! references (e.g., VERGEN_GIT_DESCRIBE check, MORPH_VERSION_SUFFIX
calculation, and VERGEN_GIT_SHA_SHORT export) refer to that single sha_short
symbol.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5f5d76ed-4c7e-48ba-8f88-d006c1550208
📒 Files selected for processing (3)
crates/node/build.rscrates/node/src/version.rsdeny.toml
✅ Files skipped from review due to trivial changes (1)
- deny.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/node/src/version.rs
Summary
Add
build.rsandsrc/version.rstomorph-nodeBefore this change,
morph-reth --versionreported the upstream reth fork's commit SHA and version:After this change:
Also bumps workspace version to
0.2.2.Changes
crates/node/build.rs— runs vergen against morph-reth's own git repocrates/node/src/version.rs—init_version_metadata()overrides reth defaultscrates/node/src/lib.rs— exposeversionmodulecrates/node/Cargo.toml— addreth-node-core,vergen,vergen-git2depsbin/morph-reth/src/main.rs— callinit_version_metadata()at startupCargo.toml— bump to 0.2.2, add vergen workspace depsSummary by CodeRabbit
New Features
Chores